Fix checklist node copying by carlobeltrame · Pull Request #8925 · ecamp/ecamp3

carlobeltrame · 2026-01-24T16:56:10Z

We had some issues when copying activities containing checklist nodes across camps.
The checklist nodes have connections to checklist items. After copying such a checklist node to a new camp, the copy still has connections to the checklist items in the old camp. In the UI this does not display, because we only display checklist items from the current camp. But in the DB and API,
here are 48 such faulty connections on prod currently.

My proposed solution is to find replacement checklist items in the target camp. I thought about only matching the text of the item, but maybe there are courses with a checklist structure like:

LS
- planen
- durchführen
- auswerten
LA
- planen
- durchführen
- auswerten

So I opted for a little more advanced algorithm, matching the item text plus optionally the hierarchical parent items, plus the checklist name if all else matches.

To reproduce:

Have a course with an activity or category with a checklist node and some selected checklist items (e.g. Quidditch in the course in the dev data)
In a separate camp, add a checklist with the same or similar items (for very similar use preset PBS Basis Wolf, for fairly similar use preset PBS Basis Pfadi)
Copy the Quidditch activity from the course to the camp and reload the page (reload is necessary due to the bug Checklist content node does not refresh when (un-)checking checklist items from a checklist that is not present yet #7816)
Before this fix, no checklist items show up in the new activity, but are present in the DB and API. After this PR, only the items with an exact match are present and visible in the new activity.

There are also some leftover Aufbau Checklist Prototypes which I now deactivated.

Previously, we only manually disabled the Basis prototypes, but forgot about the Aufbau prototypes. We can completely remove the prototypes from the dev data.

…st items

github-actions · 2026-01-24T17:20:20Z

✅ Feature branch deployment ready!

Name	Link
😎 Deployment	https://pr8925.ecamp3.ch/
🔑 Login	`test@example.com` / `test`
🕒 Last deployed at	Tue Jan 27 2026 20:40:00 GMT+0100
🔨 Latest commit	e50735c7b1279047513a51e62fc1efda1df864f1
🔍 Latest deploy log	https://github.com/ecamp/ecamp3/actions/runs/21411310229/job/61649113298
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

…ecklist items are present in target camp

BacLuc

Looks good to me as soon as the runtime of the migrations in prod is checked.

One a little mean question now that the work is already done:

We have 2 cases here:

Copy the activity in the same camp: Easy, the checklistitems are in the same camp
Copy an activity to another camp: Hard, and a perfect solution is nearly impossible.
Maybe we could just not copy the checklistnode_checklistitem relation in this case?
(virtual 3. Copy all activities and checklists from another camp: easy)

BacLuc · 2026-01-25T14:53:06Z

api/migrations/schema/Version20260124154500.php

+
+    public function up(Schema $schema): void {
+        $this->addSql(<<<'EOF'
+            DELETE FROM public.checklistnode_checklistitem


Did you run this statement as select statement in prod?
How long did it take?

Yes. For activities, it takes 1 s 951 ms and gives the result of 49 faulty connections. For categories, it takes 648 ms and gives the result of 0.

BacLuc · 2026-01-25T14:58:11Z

api/src/Entity/ContentNode/ChecklistNode.php

+
+                    /** @var ChecklistItem $existingParent */
+                    $existingParent = $existingItem->getParent();
+                    while (null !== $parent && null !== $existingParent && $score < 10) {


The 10 here is just an upper bound that the loop aborts for sure?
I think we agreed on a maximum nexting level of 3.

Maybe put it in a constant with a name?

Ah, good point. Yes, it is a failsafe. I now extracted it into a constant in 527614e.

BacLuc · 2026-01-25T15:00:14Z

api/src/Entity/ContentNode/ChecklistNode.php

+                $bestMatchIndex = array_find_key($matches, function ($match) use ($maxScore) {
+                    return $match === $maxScore;
+                });
+                if ($maxScore > 0 && null !== $bestMatchIndex) {


If there are only matches with score 1 (leaf has the same text),
but the parent checklistitem has another name, this will still lead to a match...

Yes, that is intended. This algorithm is supposed to give the best match available, not find a perfect match (because the exact wording of checklist items and their parents may vary slightly over time). I also wrote tests for this: testCopyFromPrototypeAcrossCampsSearchesForItemOfSameName

BacLuc · 2026-01-25T15:03:12Z

api/tests/Api/Activities/CreateActivityTest.php

+        $materialNodes = array_filter($contentNodes, fn ($cn) => 'Material' == $cn['contentTypeName']);
+        $this->assertCount(1, $materialNodes);
+
+        $materialNode = reset($materialNodes);


why not just use $materialNodes[0] or array_first?

I tried to find the next access to $materialNodes and why you are resetting the pointer here....

I just copied that from another test written in 9056cda and tried to be consistent. Maybe @pmattmann knows why?

carlobeltrame · 2026-01-25T15:44:33Z

One a little mean question now that the work is already done:
2. Copy an activity to another camp: Hard, and a perfect solution is nearly impossible.
Maybe we could just not copy the checklistnode_checklistitem relation in this case?

@BacLuc From a user perspective, this was the behaviour before, because the relations that were copied are never visible in the UI. I ran into this problem in a real course, and initially wanted to change it so that we can copy these connections. We already do something similar with categories (although implemented in the frontend in that case) and we go to great lengths to preserve the material items in copied activities as well. When trying to implement this similar improvement for checklist items, I discovered the bug.

Personally, I think with us supporting the use case of activity templates via publicly shared camps (#7481), the old behaviour is not satisfying. I know my proposed algorithm is not perfect with matching checklist items, particularly if there are some minor typo differences between the checklist items in both camps. I thought this could be a good starting point, and if we find more specific use cases, we can always improve the algorithm. However, if you insist on staying with the old behaviour, just not copying any connections, I can just drop the last two commits.

My ideal vision for the copy activity feature is that activities can be copied from old courses, and they "magically" adapt to the slightly changed checklist items in the new course. I think it's fair to assume that an activity from a course will probably be copied to a similar course, which probably has similar checklists (especially with the checklist templates). I imagine (but have no hard evidence for) that this scenario is what happens most of the time, and IMO we should make it easy to work in this case.

manuelmeister

Core Meeting Decision

We agree that this makes sense. Thanks!

carlobeltrame added 4 commits January 24, 2026 14:42

Fix typo

a39b542

Don't connect checklist nodes to checklist items from another camp

8f579ec

Disable all ecamp-managed checklist prototypes

ff973f0

Previously, we only manually disabled the Basis prototypes, but forgot about the Aufbau prototypes. We can completely remove the prototypes from the dev data.

Remove invalid cross-camp connections from checklist nodes to checkli…

66c1d35

…st items

carlobeltrame requested a review from a team January 24, 2026 16:56

carlobeltrame added the deploy! Creates a feature branch deployment for this PR label Jan 24, 2026

carlobeltrame temporarily deployed to feature-branch January 24, 2026 17:20 — with GitHub Actions Inactive

carlobeltrame force-pushed the fix-checklist-node-copying branch from cba8d96 to a2aa911 Compare January 24, 2026 22:18

carlobeltrame temporarily deployed to feature-branch January 24, 2026 22:21 — with GitHub Actions Inactive

carlobeltrame force-pushed the fix-checklist-node-copying branch from a2aa911 to ef948aa Compare January 25, 2026 09:13

carlobeltrame temporarily deployed to feature-branch January 25, 2026 09:16 — with GitHub Actions Inactive

carlobeltrame force-pushed the fix-checklist-node-copying branch from ef948aa to c5fe5e0 Compare January 25, 2026 13:27

carlobeltrame temporarily deployed to feature-branch January 25, 2026 13:28 — with GitHub Actions Inactive

Only copy the checklistNode-checklistItem connections when similar ch…

1643a02

…ecklist items are present in target camp

carlobeltrame force-pushed the fix-checklist-node-copying branch from c5fe5e0 to 1643a02 Compare January 25, 2026 13:30

carlobeltrame temporarily deployed to feature-branch January 25, 2026 13:31 — with GitHub Actions Inactive

BacLuc approved these changes Jan 25, 2026

View reviewed changes

Extract max checklist item nesting depth into a constant and reuse it

527614e

carlobeltrame temporarily deployed to feature-branch January 25, 2026 15:29 — with GitHub Actions Inactive

carlobeltrame requested a review from a team January 25, 2026 16:37

manuelmeister requested review from Copilot and manuelmeister and removed request for a team and Copilot January 26, 2026 07:17

Copilot started reviewing on behalf of manuelmeister January 26, 2026 07:18 View session

pmattmann approved these changes Jan 27, 2026

View reviewed changes

manuelmeister approved these changes Jan 27, 2026

View reviewed changes

Also distinguish by checklist prototype id

e50735c

carlobeltrame enabled auto-merge January 27, 2026 19:35

carlobeltrame temporarily deployed to feature-branch January 27, 2026 19:38 — with GitHub Actions Inactive

carlobeltrame added this pull request to the merge queue Jan 27, 2026

Merged via the queue into ecamp:devel with commit 3c1a999 Jan 27, 2026
35 checks passed

carlobeltrame deleted the fix-checklist-node-copying branch January 27, 2026 19:49

This was referenced Jan 27, 2026

Deploy to staging #8940

Merged

Deploy to prod #8942

Merged

Comments

Conversation

carlobeltrame commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Feature branch deployment ready!

Uh oh!

BacLuc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BacLuc Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

carlobeltrame Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

BacLuc Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

carlobeltrame Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

BacLuc Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

carlobeltrame Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BacLuc Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

carlobeltrame Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

carlobeltrame commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

manuelmeister left a comment

Choose a reason for hiding this comment

Core Meeting Decision

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

carlobeltrame commented Jan 24, 2026 •

edited

Loading

github-actions bot commented Jan 24, 2026 •

edited

Loading

BacLuc left a comment •

edited

Loading

carlobeltrame Jan 25, 2026 •

edited

Loading

carlobeltrame commented Jan 25, 2026 •

edited

Loading